Skip to content
This repository was archived by the owner on May 6, 2020. It is now read-only.

ref(Dockerfile): ditch the pip cache for a slightly smaller image#1051

Merged
helgi merged 1 commit intodeis:masterfrom
helgi:ditch_pip_cache
Sep 9, 2016
Merged

ref(Dockerfile): ditch the pip cache for a slightly smaller image#1051
helgi merged 1 commit intodeis:masterfrom
helgi:ditch_pip_cache

Conversation

@helgi
Copy link
Copy Markdown
Contributor

@helgi helgi commented Sep 9, 2016

No description provided.

@helgi helgi added this to the v2.6 milestone Sep 9, 2016
@helgi helgi self-assigned this Sep 9, 2016
@deis-bot
Copy link
Copy Markdown

deis-bot commented Sep 9, 2016

@krancour, @bacongobbler and @mboersma are potential reviewers of this pull request based on my analysis of git blame information. Thanks @helgi!

Comment thread rootfs/Dockerfile
rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* /usr/share/man /usr/share/doc
curl -sSL https://bootstrap.pypa.io/get-pip.py | python - pip==8.1.2 && \
mkdir -p /configs && chown -R deis:deis /configs && \
pip install --disable-pip-version-check --no-cache-dir -r /app/requirements.txt && \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't --no-cache-dir already do this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd think but if you drop onto the controller container and dh -h (or similar) that dir, it has data. Can you try it on yours just to confirm?

@helgi
Copy link
Copy Markdown
Contributor Author

helgi commented Sep 9, 2016

Also considering removing pip itself

@Joshua-Anderson
Copy link
Copy Markdown
Contributor

Removing pip makes sense. Any reason not to?

@helgi
Copy link
Copy Markdown
Contributor Author

helgi commented Sep 9, 2016

Mostly if we are extending main to dev would be a reason to keep it but that's about it!

@codecov-io
Copy link
Copy Markdown

Current coverage is 86.87% (diff: 100%)

Merging #1051 into master will not change coverage

@@             master      #1051   diff @@
==========================================
  Files            42         42          
  Lines          3565       3565          
  Methods           0          0          
  Messages          0          0          
  Branches        607        607          
==========================================
  Hits           3097       3097          
  Misses          309        309          
  Partials        159        159          

Powered by Codecov. Last update f10d0a3...4e51970

@Joshua-Anderson
Copy link
Copy Markdown
Contributor

LGTM, but we should probably investigate why this is neccessary (maybe open an issue?). Could it be that --no-cache only partially disables caching?

@helgi helgi merged commit 13de6e6 into deis:master Sep 9, 2016
@helgi helgi deleted the ditch_pip_cache branch September 9, 2016 21:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants